NetNews Offline 2/NetNews Offline Volume 2.iso/news/comp/lang/c-part1/2073

Text File | 1996-08-05 | 6.1 KB | 232 lines

Path: mail2news.demon.co.uk!genesis.demon.co.uk
From: Lawrence Kirby <[e-mail]>
Newsgroups: comp.lang.c
Subject: Re: Problem with program
Date: Thu, 18 Jan 96 21:23:48 GMT
Organization: none
Message-ID: <[e-mail]>
References: <4dkr5u$[e-mail]>
Reply-To: [e-mail]
X-NNTP-Posting-Host: genesis.demon.co.uk
X-Newsreader: Demon Internet Simple News v1.27
X-Mail2News-Path: genesis.demon.co.uk

In article <4dkr5u$[e-mail]> [e-mail] "Rune Elvemo" writes:

>Anyone who can figure out what is wrong with this program?
>
>I tried to compile it, but my compiler wouldn't.....
>
>Ex: It said that the prototype needed a semicolon, but it
>don't.........
>
>
>
>/* Loadfile.c - loads a txt file, and shows it
>**
>**
>*/
>
>#include <stdio.h>

Since you use calloc below you need to declare it e.g. by:

#include <stdlib.h>

>
>struct Text
>{
>char *str;
>struct Text *next;
>}

The ; is needed here.

>enum BOOL { TRUE, FALSE };
>
>/* prototype */
>void PText(struct Text *);
>
>
>main(int argc, char *argv[])

If this had come straight after your struct Text declaration the compiler
would have interpreted struct Text as the return type for main (and probably
not issued a warning). It is therefore better to write this fiully as:

int main(int argc, char *argv[])

>{
>BOOL done = FALSE;

In C this must be:

enum BOOL done = FALSE;

>FILE *txtfile;
>struct Text *first, *tst;
>
>if (argc >1)
>     {
>     if (txtfile = fopen(argv[1], "r"))

Many compilers will generate a 'helpful' warning about this just in case
you mean == rather than =. It is a normal convention to 'reassure' the
compiler (and anybody else reading your code for that matter) to write it
as

      if ((txtfile = fopen(argv[1], "r")))

or

      if ((txtfile = fopen(argv[1], "r")) != NULL)

>          {
>          if(first = calloc(1, sizeof(struct Text)))

The idea of calloc is to allocate an object from the heap and also
initialise it to all bits zero. The only types that can be safely
initialised this way are integer ones. You have no guarantee from the
C language that all bits zero corresponds to 0.0 or NULL for floating point
and pointer types. If you don't need to initialise the object then use
malloc.

>               {
>               tst = first;
>               if (fgetc(txtfile) != -1)

fgetc returns EOF on an end-of-file or error condition. The language
guarantees that EOF is less than zero but it does not guarantee that
it is -1. So make that:

                if (fgetc(txtfile) != EOF)

>                    {
>                    while (!done)
>                         {
>                         if (tst->str = calloc(256, sizeof(char)))

sizeof(char) is defined to be 1 by the C language although there's nothing
particularly wrong with stating it explicitly. It looks like you don't
need the initialisation here so could use malloc(256) instead.

>                              {
>                              do
>                                   {
>                                   *tst->str = fgetc(txtfile);
>                                   tst->str++;
>                                   *tst->str = 0;
>/* check if the last char was LineFeed or EOF  */
>
>                                   } while ((*(tst->str - 1) != -1) &&
>(*(tst->str) != 10));
           ^^ -1 ???

Linefeed (end-of-line) is '\n' in C. On many implementations it has the
value 10 but the language doesn't guarantee that. Also you have no
guarantee that a char can store the value of EOF (which is why fgetc
returns an int). You may be looking for something like:

                               while ((ch = fgets(txtfile)) != EOF &&
                                                                 ch != '\n')
                                    *tst->str++ = ch;

                               *tst->str = '\0';

Where ch was previously defined as an int. But note that like your code
this doesn't test of writing beyond the end of the allocated buffer which
it really should do (left as an exercise! :-) )

>                              if (*tst->str == -1)
>                                   done = TRUE;

EOF again.

>                              else
>                                   {
>                                   if (tst->next = calloc(1,
>sizeof(struct Text)))
>                                        {
>                                        tst = tst->next;
>
>                                        }
>                                   else
>                                        {
>                                        done = TRUE;
>                                        printf("\n**!!Not enough
>memory!!**\n");
>                                        }
>                                   }
>                              }
>                         }
>                    tst->next = calloc(1, sizeof(struct Text));
>
>
>                    PText(first);
>                    }
>               }
>          else
>               printf("\n**!!Not enough memory!!**\n");
>          }
>     }

Since main returns an int:

      return 0;
>}
>
>/* Print the text that we have got */
>void PText(struct Text *txt)
>{
>BOOL done = FALSE;

enum BOOL done = FALSE;

>struct Text *jump;
>
>jump = txt;
>
>while(!done)

Done doesn't seem to be set TRUE anywhere.

>     {
>     printf("%s", jump->str);
>
>     while (*txt->next != 0)

You are trying to compare a structure against 0 which is illegal. You want
to compare the pointer e.g.

      while (jump->next != NULL)

I assume you wanted to compare the link in the current element, not the
first each time!

>          {
>          jump = jump->next;
>
>          printf("%s\n", jump);

           printf("%s\n", jump->str);

>          }
>     }
>}

You may prefer:

void PText(const struct Text *txt)
{
    const struct Text *jump;

    for (jump = txt; jump != NULL; jump = jump->next)
        printf("%s\n", txt->str);
}

--
-----------------------------------------
Lawrence Kirby | [e-mail]
Wilts, England | [e-mail]
-----------------------------------------